Add concept of block events and some design notes #1059
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1059 +/- ##
==========================================
+ Coverage 74.28% 74.38% +0.10%
==========================================
Files 66 66
Lines 4487 4556 +69
==========================================
+ Hits 3333 3389 +56
- Misses 1154 1167 +13
🚀 New features to boost your workflow:
|
ml-evs
force-pushed
the
ml-evs/blocks
branch
2 times, most recently
from
2f892ca to
f765a97
Compare
|
Just pinging @BenjaminCharmes and @jdbocarsly for review on this one! |
| @@ -27,9 +27,19 @@ class XRDBlock(DataBlock): | |||
| def plot_functions(self): | |||
| return (self.generate_xrd_plot,) | |||
|
|
|||
| @classmethod | |||
| @event | |||
| def set_wavelength(self, wavelength: float | None): | |||
There was a problem hiding this comment.
Can you show how this gets implemented in the XRD block before we merge? That would be a good way to test that everything is working on the front end
| @@ -207,6 +207,10 @@ export default { | |||
| this.contentMaxHeight = "none"; | |||
| } | |||
| }); | |||
| document.addEventListener("bokehStateUpdate", this.handleBokehEvent); | |||
There was a problem hiding this comment.
If this adds a listener to the overall document, will we have multiple listeners if we have multiple blocks on a page? This could cause the handler to be called multiple times
you should be able to add this listener to a only the relevant node by assigning the outer div ref="thisBlock", then only add the listener to:
this.$refs.thisBlock.addEventListener("bokehStateUpdate", this.handleBokehEvent);
There was a problem hiding this comment.
Still struggling a lot with this... following the approach with $refs, it seems that the block events bubble up to the block-list-item defined in the EditPage and skip the DatablockBase.
There was a problem hiding this comment.
Perhaps the easiest resolution is adding a ref to the DataBlockBase and then directly calling:
<template>
<DataBlockBase :item_id="item_id" :block_id="block_id" ref="xrdBlockBase">
</template>
...
this.$refs.xrdBlockBase.handleBokehEvent(event);
|
Made some fixes post-discussion with @jdbocarsly. Known limitations still:
|
ml-evs
force-pushed
the
ml-evs/blocks
branch
2 times, most recently
from
f7fc272 to
34c71dd
Compare
ml-evs
force-pushed
the
ml-evs/blocks
branch
2 times, most recently
from
827928a to
3f6c7e2
Compare
This PR adds the concept of "events" that can be registered on block types; essentially, server-side functions that can be triggered separately from parsing/ingestion for things like setting parameters. It also introduces the handling of particular events raised by Bokeh plots in the base data block in Vue.
This is all described in a new design document for this current version of datablocks, also committed in this PR.
I think eventually we can add several lifecycle hooks to the base block for handling certain things (remake block from scratch etc) that can be reused by derived block types, though I haven't put any effort into doing this now. I'm already using this functionality in a custom block that sets some parameters based on user feedback that requires reanalysing all block data (essentially a slider setting some offset time parameter).
For review, I recommend reading the design doc in the rtd preview at https://the-datalab--1059.org.readthedocs.build/en/1059/design/blocks/.
There's still a few open questions about how errors in events should be handled, and what methods should be triggered afterwards (mostly due to the fact we always call the
to_web()block method in the update-block route.